Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Parent FlagSet #1795

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add Parent FlagSet #1795

wants to merge 1 commit into from

Conversation

fahedouch
Copy link

@fahedouch fahedouch commented Sep 5, 2022 •
edited
Loading

support pFlag AddParentFlagSet (spf13/pflag#357)
Signed-off-by: fahed dorgaa [email protected]

@fahedouch
Add Parent FlagSet
0c653d6 
Signed-off-by: fahed dorgaa <[email protected]>
@fahedouch fahedouch marked this pull request as draft September 5, 2022 13:39
@github-actions github-actions bot added the size/XS Denotes a PR that changes 0-9 lines label Sep 5, 2022
@fahedouch fahedouch marked this pull request as ready for review September 5, 2022 22:33
@fahedouch fahedouch mentioned this pull request Sep 5, 2022
Closed
AkihiroSuda
AkihiroSuda reviewed Sep 5, 2022
View reviewed changes
command.go
@@ -1672,7 +1672,7 @@ func (c *Command) Parent() *Command {
func (c *Command) mergePersistentFlags() {
c.updateParentsPflags()
c.Flags().AddFlagSet(c.PersistentFlags())
c.Flags().AddFlagSet(c.parentsPflags)
c.Flags().AddParentFlagSet(c.parentsPflags)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs unit testing.

Also, not sure whether changing the default behavior is allowed. Probably this should be configurable.

@fahedouch fahedouch marked this pull request as draft September 10, 2022 22:12
jpmcb
jpmcb reviewed Oct 1, 2022
View reviewed changes
Copy link
Collaborator

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you give a broader description of this change and what use case it's trying to solve?

Also, I'm assuming this is a subset of this PR? spf13/pflag#357 We would need to wait for a new pflag release and then consume it from the upstream.

@jpmcb jpmcb added the triage/needs-info Needs more investigation from maintainers or more info from the issue provider label Oct 1, 2022
@github-actions
Copy link

github-actions bot commented Dec 1, 2022

The Cobra project currently lacks enough contributors to adequately respond to all PRs. This bot triages issues and PRs according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied. - After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied and the PR is closed.
    You can:
  • Make a comment to remove the stale label and show your support. The 60 days reset. - If a PR has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interested in reopening.

@github-actions github-actions bot closed this Jan 1, 2023
@marckhouzam
Copy link
Collaborator

marckhouzam commented Dec 23, 2023 •
edited
Loading

As we have very few maintainers, and those have very little time, we've decided to remove the auto-close configuration.
I'm re-opening this PR since it was closed automatically.

@marckhouzam marckhouzam reopened this Dec 23, 2023
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpmcb jpmcb jpmcb left review comments

@AkihiroSuda AkihiroSuda AkihiroSuda left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
size/XS Denotes a PR that changes 0-9 lines triage/needs-info Needs more investigation from maintainers or more info from the issue provider
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@fahedouch @marckhouzam @CLAassistant @AkihiroSuda @jpmcb